Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start Tcl design inspection methods #4678

Merged
merged 7 commits into from
Dec 9, 2024
Merged

Conversation

povik
Copy link
Member

@povik povik commented Oct 21, 2024

What are the reasons/motivation for this change?

Allow inspection and altering of the design from Tcl scripts for use-cases which are too specific to write a Yosys command for.

This is in addition to the existing ways of obtaining information about a design by

[yosys tee -q -s result.json stat -json]

and variations thereof.

See sample usage here: https://github.com/povik/OpenROAD-flow-scripts/blob/72b084f68275aaefec903fbc8e4f8613679e52e0/flow/scripts/synth_preamble.tcl#L122

Another use case of the new API is replacing this Python script called from Tcl to process a JSON export with a direct Tcl implementation: https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/flow/scripts/mem_dump.py (Cc @oharboe, feel free to comment)

Explain how this is achieved.

We introduce a set of Tcl functions prefixed with rtlil:: to query attributes and parameters.

Tcl bindings in other projects sometimes use text-formatted pointers as the Tcl-side handles, but for a RTLIL binding we can consistently use IDs of objects to refer to them from the Tcl side.

@povik povik marked this pull request as ready for review November 4, 2024 15:19
Copy link
Member

@jix jix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this lgtm, I'd suggest making a decision on flag / option parsing before merging a first version, and also fixing the unsigned MSB set int handling, both to avoid backwards incompatible changes. The other points I commented on can be addressed in a follow up if the goal is to get something usable merged quickly.

kernel/tclapi.cc Outdated Show resolved Hide resolved
kernel/tclapi.cc Outdated Show resolved Hide resolved
kernel/tclapi.cc Outdated
(!mod_flag && i != argc - 3) ||
(string_flag + int_flag + bool_flag > 1))
ERROR("bad usage: expected \"get_attr -mod [-string|-int|-bool] <module> <attrname>\""
" or \"get_attr [-string|-int|-bool] <module> <identifier> <attrname>\"")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to support an optional -default <default_value> (which will return the default value unchanged, not reinterpret it as the selected type).

One potential issue with adding more flags (whether it's this -default or something else) is that before doing so get_attr -mod -default some_attr would look up some_attr in a module -default but after adding that option it would be missing arguments. Maybe we should require using {\-default} (which already works due to escape_id) and/or -- and interpret all - strings as flags/options (producing an error if unknown) until -- is found. This is also an issue just for the existing flags, i.e. you have to use [rtlil::get_attr -mod -string {\-mod} some_attr] and can't use [rtlil::get_attr -mod -string -mod some_attr] to access module \-mod (... . I'm not sure what's the convention for tcl commands wrt to disambiguating options and strings starting with -.

kernel/tclapi.cc Outdated
Tcl_CreateCommand(interp, "rtlil::has_attr", tcl_has_attr, NULL, NULL);
Tcl_CreateCommand(interp, "rtlil::set_attr", tcl_set_attr, NULL, NULL);
Tcl_CreateCommand(interp, "rtlil::get_param", tcl_get_param, NULL, NULL);
Tcl_CreateCommand(interp, "rtlil::set_param", tcl_set_param, NULL, NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also expect rtlil::unset_attr as well as rtlil::attr_list to get a complete set of operations (same for param which is also missing rtlil::has_param).

Maybe also useful would be something like rtlil::attr_array which would produce all attributes as a key-value array such that you can do array set object_attrs [rtlil::attr_array some_mod some_object]; puts "the value of keep is $object_attrs(\\keep)" and use other tcl array functions. I don't actually use tcl enough to know how useful this would be, though.

@povik
Copy link
Member Author

povik commented Dec 5, 2024

Pulled fixes from @jix. Thanks!

@povik povik merged commit b0708a3 into YosysHQ:main Dec 9, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants